-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: CURL option force_ip_resolve
#9194
base: 4.6
Are you sure you want to change the base?
Conversation
force_ip_resolve
system/HTTP/CURLRequest.php
Outdated
if (array_key_exists('force_ip_resolve', $config) && $config['force_ip_resolve']) { | ||
$protocolVersion = $config['force_ip_resolve']; | ||
|
||
if ($protocolVersion === 'v4') { | ||
$curlOptions[CURLOPT_IPRESOLVE] = CURL_IPRESOLVE_V4; | ||
} elseif ($protocolVersion === 'v6') { | ||
$curlOptions[CURLOPT_IPRESOLVE] = CURL_IPRESOLVE_V6; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about:
if (array_key_exists('force_ip_resolve', $config) && $config['force_ip_resolve']) { | |
$protocolVersion = $config['force_ip_resolve']; | |
if ($protocolVersion === 'v4') { | |
$curlOptions[CURLOPT_IPRESOLVE] = CURL_IPRESOLVE_V4; | |
} elseif ($protocolVersion === 'v6') { | |
$curlOptions[CURLOPT_IPRESOLVE] = CURL_IPRESOLVE_V6; | |
} | |
} | |
if (array_key_exists('force_ip_resolve', $config)) { | |
$curlOptions[CURLOPT_IPRESOLVE] = match($config['force_ip_resolve']) { | |
'v4' => CURL_IPRESOLVE_V4, | |
'v6' => CURL_IPRESOLVE_V6, | |
default => CURL_IPRESOLVE_WHATEVER | |
}; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or throw for unknown value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... yes, that's probably even better than silently selecting the default value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think dont any insert option when no call, better throw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This options same behaviour in version
. If there no set declaration, we follow default of PHP Curl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if I initialize this service with custom options? I can't change CURLOPT_IPRESOLVE
to the default value later?
This seems like an issue - same story with version
. There is no option to reset it to the default value during the instance lifecycle.
b6a6f7a
to
b02bdf7
Compare
Description
I think this needed for resolve only
ipv4
oripv6
.Checklist: